Separate :root, light-theme and dark-theme styles. #8295
Conversation
|
/cc @parlough too, not sure why I couldn't add you as a reviewer |
jonasfj
left a comment
jonasfj
left a comment
There was a problem hiding this comment.
So what if we ever need variables that are specific to dark or light mode?
(I don't mind tests enforcing this structure, but they should provide a good message when the fail, so that you don't have to figure out what the test does first).
|
I see value in trying this out, as the thought of forgetting to define a corresponding dark mode color did cross my mind. The one downside is that there is no fallback if the class is somehow missing, so tests should likely also make sure each page is rendered with one of the classes added. |
Not sure how that would work, maybe those should be in shared?
Yes, this is a good point. I will add this test to this PR and add further tests for the variable splits as a separate PR. |
|
An alternative idea: we could keep the :root rule, but still separate the variables into two groups: one that is checked that they should have a matching dark-theme variant, and the other that is checked that they don't have further overrides. It could provide the value of having tests and separation, but still doing a good fallback in case we miss something. |
isoos
force-pushed
the
style-separation
branch
from
15b2a10 to
7ed8142
Compare
3 participants
:rootand.light-theme.:root, and override them under.dark-theme. I think it may be worth considering the separation of:rootto have only shared variables that are not redefined in later scopes, and the two theme have variables that are in both scopes.